Skip to content

Remove dependencies on org.apache.commons.jxpath #2775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 3 commits into from
Closed

Remove dependencies on org.apache.commons.jxpath #2775

wants to merge 3 commits into from

Conversation

@merks
Copy link
Contributor

@merks merks commented Feb 5, 2025

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 5, 2025 •
edited
Loading

Test Results

 1 818 files   - 3   1 818 suites   - 3   1h 28m 46s ⏱️ - 4m 43s
 7 716 tests  - 3   7 487 ✅  -  4  228 💤 ±0  1 ❌ +1 
24 309 runs   - 9  23 559 ✅  - 10  749 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit f9c86d3. ± Comparison against base commit bd93b33.

This pull request removes 4 and adds 1 tests. Note that renamed tests count towards both. 
org.eclipse.e4.emf.xpath.test.EDynamicPropertyHandlerTest ‑ test_getProperty
org.eclipse.e4.emf.xpath.test.EDynamicPropertyHandlerTest ‑ test_getPropertyNames
org.eclipse.e4.emf.xpath.test.EDynamicPropertyHandlerTest ‑ test_getProperty_When_UnknownProperty_Expect_NullResult
org.eclipse.e4.emf.xpath.test.EDynamicPropertyHandlerTest ‑ test_setProperty

org.eclipse.e4.emf.xpath.test.ExampleQueriesTestCase ‑ testRelative

♻️ This comment has been updated with latest results.

@merks
Improve the implementation of JavaXPathContextFactoryImpl
f9c86d3 
- Reuse EMF's support for serializing directly to DOM which directly
supports maintaining the required mappings.
-  Add ExampleQueriesTestCase.testRelative() to test relative paths via
a nested context.
- Also verify that in ExampleQueriesTestCase.testMenuQuery the query
`//.[@id='menu.1'` is behaving correctly relative to the target example
model.

eclipse-platform#423
@jukzi
Copy link
Contributor

jukzi commented Feb 6, 2025

Removing dependencies is a good thing, thanks! :-)

@merks
Copy link
Contributor Author

merks commented Feb 6, 2025

Removing dependencies is a good thing, thanks! :-)

Moreover this dependency is not well-maintained, i.e., not maintained at all, it has a rejected CVE associated with it that never really wants to die, and it drags in other fossilized content:

https://repo1.maven.org/maven2/org/jdom/jdom/1.1.3/

It really needs to go away!!

This was referenced Feb 9, 2025
Merged
Merged
@HannesWell
Copy link
Member

HannesWell commented Feb 9, 2025

Thank you Ed for your suggestions, I'm currently incorporating them into #2290 as you suggested in our (private ) conversation.

@HannesWell
Copy link
Member

HannesWell commented Feb 11, 2025

With #2290 being submitted, this is not necessary anymore.

@HannesWell HannesWell closed this Feb 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@merks @jukzi @HannesWell @ptziegler